-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve validation step info to string for hashing #511
Conversation
Also |
I think this should do the trick! Some notes on the implementation:
agent1 <- create_agent(small_table) %>%
rows_distinct() %>%
interrogate()
agent1$validation_set$sha1
#> [1] "adac947b3a26f7df4bfd374b5fff4e67b4190f8b-v0.12"
agent1$validation_set$sha1 <- "bad hash"
agent2 <- rehash_agent(agent1)
agent2$validation_set$sha1
#> [1] "adac947b3a26f7df4bfd374b5fff4e67b4190f8b-v0.12"
agent1$validation_set$sha1
#> [1] "bad hash"
agent2$validation_set$sha1
#> [1] "adac947b3a26f7df4bfd374b5fff4e67b4190f8b-v0.12"
multiagent <- create_multiagent(agent1, agent2)
multiagent$agents[[1]]$validation_set$sha1
#> [1] "adac947b3a26f7df4bfd374b5fff4e67b4190f8b-v0.12"
multiagent$agents[[2]]$validation_set$sha1
#> [1] "adac947b3a26f7df4bfd374b5fff4e67b4190f8b-v0.12"
get_multiagent_report(multiagent, display_mode = "wide") All in all, this fixes both the alignment issue (#509) and the performance issue (#508). There are no user visible changes beyond "bugfix in multiagent alignment", so the news item for this PR just reports that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This is really great work! Feel free to merge any time. |
Summary
This PR introduces a new internal function
hash_validation_step()
which ensures that functions (inpreconditions
) and quosures (vars()
invalue
/left
/right
) are resolved to character for the purposes of hashing. In turn,digest::sha1()
is guaranteed to always receive a character vector - this resolves issues with environment hashing as described in #509.Currently, the new hashing implementation replaces the previous one (vs. being opt-in).
Pending
Backwards compatibility for older agents in the multiagent: we would need to re-hash agents' steps for the purposes of alignment.
More tests to check that this new method does not create false positives in step alignments. Alignment should respect identity up to yaml-equivalence.